Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement LLE-GCM #3096

Merged
merged 4 commits into from Aug 1, 2017
Merged

Implement LLE-GCM #3096

merged 4 commits into from Aug 1, 2017

Conversation

jarveson
Copy link
Contributor

@jarveson jarveson commented Jul 26, 2017

Implements support for lle of GCM.

  • LLE turned to auto / default as major regression seems to be found and fixed
  • HLE still functions as well as it did (with some fixes), but will need to be manually coded out
  • Report any regressions found with lle now

@mention-bot
Copy link

@jarveson, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Nekotekina, @vlj and @acmiyaguchi to be potential reviewers.

if (!m_sysrsx)
fmt::throw_exception("sys_rsx_context_allocate called twice.");

vm::falloc(0x40000000, 0x10000000, vm::rsx_context);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem necessary to allocate 256 MB for this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no one else can use it in the PS3, it doesn't matter, the OS will lazy alloc it anyway...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On linux - maybe. You normally shouldn't rely on such an idea.

Copy link
Contributor

@Farseer2 Farseer2 Jul 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Windows, too, its saved in a VAD.
Anyway, I'd agree with you on other types of applications, but as this is virtual memory on virtual memory, I believe it'll faster than checking if you need to alloc more in realtime. It isn't wasted memory, too, because it isn't used and no one will access it, so it shouldn't page fault.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea ill resize it to something smaller

@@ -783,6 +783,7 @@ namespace vm
{
std::make_shared<block_t>(0x00010000, 0x1FFF0000), // main
std::make_shared<block_t>(0x20000000, 0x10000000), // user
std::make_shared<block_t>(0x40000000, 0x30000000), // rsx contexts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this at the end of the list. Why 0x30000000?

Copy link
Contributor Author

@jarveson jarveson Jul 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lv1/2 allows 3 rsx contexts i think, with vsh normally at 0x60000000, game context is at 0x40000000, so i assume it gives the full 0x10000000 for each context. Also, why end of list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put this down to 0x10000000 because it seemed to force sonic unleashed to be unable to vm_map with it taking so much memory, may have to be looked into what actually is used by ps3 then

@@ -16,6 +16,7 @@ namespace vm
{
main,
user_space,
rsx_context,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code very much depends currently on being able to get the 0x40000000 allocation for context, unless there is a better way to do it that im missing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this. First 4 entries are special and they aren't required to allocate something with falloc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooh, i see now, k ill make the changes

@Xcedf
Copy link

Xcedf commented Jul 27, 2017

Now this branch even greater after being able to swithch between LLE and HLE gcm, but lle-gcm had some issues and hle also has it now
what kind of issue could this be
27
28
Some other games also has the same
from my list its
SH Downpour
Bulletstorm
Spec Ops The Line
And btw, will be gcm module removed from lv2 ignore list?

@jarveson jarveson mentioned this pull request Jul 27, 2017
@jarveson
Copy link
Contributor Author

jarveson commented Jul 27, 2017

@Xcedf are these regressions with HLE? For now ive opened issue #3105 to track issues like the ones you've mentioned with lle, although if they happen with HLE now, that changes things

@Xcedf
Copy link

Xcedf commented Jul 27, 2017

@jarveson yes, they happen with hle now too

@jarveson
Copy link
Contributor Author

Fixed zcull issue which should remove regressions that happened

@Xcedf
Copy link

Xcedf commented Jul 27, 2017

Confirm fixed now
28

@Xcedf
Copy link

Xcedf commented Jul 28, 2017

fixed screen update issue in Metal Gear Revengeance
33

@AniLeo
Copy link
Member

AniLeo commented Jul 31, 2017

Any other regressions? Should be good for merge 🔜 if not

@Illynir
Copy link

Illynir commented Jul 31, 2017

I don't know if this a regression but in Kingdom Heart 1 on KH 1.5 Remix HD the shadows of characters flicker a little. On master, they aren't flicker at all and are perfect. (With write color buffer else there are no shadows at all on master and LLE GCM)

Then... I don't know if it's because write color buffer in combination with LLE GCM or not or if it's really a regression.

For the rest, no problem to report.

@Igihara
Copy link

Igihara commented Jul 31, 2017

Controlled character moves slow when the analog stick is moved diagonally on this build(?) using xinput. Blog build doesn't have that issue.

@AniLeo
Copy link
Member

AniLeo commented Jul 31, 2017

This build doesn't touch XInput and the blog build is not a master

@Inviuz
Copy link
Contributor

Inviuz commented Jul 31, 2017

Probably related to recent xinput changes, try changing Pad Squircling Factor in config_xinput.yml to 4000

@fifazalata
Copy link

@Igihara If you mean running diagonally in P5, then try changing
Pad Squircling Factor: from 8000 to 4000 in the "config_xinput.yml" file
That should fix the problem.

@Igihara
Copy link

Igihara commented Aug 1, 2017

Changing it to 4000 fixed it. Thanks guys.

@AniLeo AniLeo requested a review from Nekotekina August 1, 2017 17:57
@Nekotekina Nekotekina merged commit 02845f5 into RPCS3:master Aug 1, 2017
@jarveson jarveson deleted the lle-gcm-pr2 branch August 2, 2017 01:34
@Kubokoo
Copy link

Kubokoo commented Aug 2, 2017

In master version with this pull, persona 5 in menus there is some black piece of that covers text.
(RPCS3 v0.0.3-8-b396826 Alpha
Intel(R) Core(TM) i5-4670K CPU @ 3.40GHz | 4 Threads | 15.69 GiB RAM | AVX)

image

In older version (RPCS3 v0.0.3-5587-5b61dcdc Alpha) this problem does not exists.

image

@jarveson
Copy link
Contributor Author

jarveson commented Aug 3, 2017

try with #3135 @Kubokoo

@Kubokoo
Copy link

Kubokoo commented Aug 3, 2017

On a freshly compiled version (without #3135 ) I got this:

image

With #3135 I got this (there still is some black part, but it does not cover text anymore):

image

@Kravickas
Copy link
Contributor

Kravickas commented Aug 7, 2017

This broke NHL 15 to not rendering anything

I think it cos u deleted

-		/*
-
-		// Store previous fbo addresses to detect RTT config changes.
-		std::array<u32, 4> m_previous_color_address = {};
-		u32 m_previous_address_z = 0;
-		u32 m_previous_target = 0;
-		u32 m_previous_clip_horizontal = 0;
-		u32 m_previous_clip_vertical = 0;
-		*/

2 previous build were getting E {rsx::thread} RSX: FBO check failed: 0x8cd6, but it was rendering game.

@Kravickas Kravickas mentioned this pull request Aug 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet